Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves https://github.com/pypr/cyarray/issues/13 #14

Merged
merged 6 commits into from
Aug 18, 2023

Conversation

nauaneed
Copy link
Contributor

I put together a fix for #13. This might be unclean, but cyarray installs now. Could work until there is a better fix, maybe?

partial fix for pypr#13
thanks prajwal
cython>3.0 disallows setting `ndarray.data=<char *>some_data`.

I believe `ndarray.data=<char *>some_data` is being used in the first
place because it is more efficient than using
`PyArray_SimpleNewFromData`, maybe.

So, this cython restriction is overcome with a
fix from https://github.com/rainwoodman/pandas/blob/05d3fe2402e4563124e7060837ded7513ab5bca7/pandas/_libs/reduction.pyx#L27

interim partial fix for pypr#13
@nauaneed nauaneed force-pushed the fix-cy3 branch 6 times, most recently from 575e6ad to a2bbae1 Compare August 17, 2023 13:14
@nauaneed nauaneed changed the title Fix https://github.com/pypr/cyarray/issues/13 Resolves #13 Aug 17, 2023
@nauaneed nauaneed changed the title Resolves #13 Resolves https://github.com/pypr/cyarray/issues/13 Aug 17, 2023
@nauaneed nauaneed changed the title Resolves https://github.com/pypr/cyarray/issues/13 resolves https://github.com/pypr/cyarray/issues/13 Aug 17, 2023
@nauaneed nauaneed changed the title resolves https://github.com/pypr/cyarray/issues/13 Resolves https://github.com/pypr/cyarray/issues/13 Aug 17, 2023
Copy link
Contributor

@prabhuramachandran prabhuramachandran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, barring the change to the cimport. Please resolve it and I will merge this. Many thanks.

@@ -43,7 +43,7 @@ IF UNAME_SYSNAME == "Windows":
ELSE:
from libc.stdint cimport uintptr_t

cimport numpy as np
cimport numpy as npqq
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why npqq?

`python setup.py` fails on macos and windows so using `pip install .`
- these changed were motivated by deprecation warnings in workflow run
  logs
@prabhuramachandran prabhuramachandran merged commit a2c6f16 into pypr:master Aug 18, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants